-
Notifications
You must be signed in to change notification settings - Fork 2k
Feat/kubectl rollout history daemonset #1818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/kubectl rollout history daemonset #1818
Conversation
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
final String directiveMarker = "$patch"; | ||
Object directive = templateMap.get(directiveMarker); | ||
if (directive == null) { | ||
throw new IllegalArgumentException("not a directive patch"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think the SMP directives are mandatory in the patch body, and as SSA(Server) goes GA, we should switch all the usage of SMP to SSA to get rid of these magic markers. can you add a TODO comment over here saying that we prefer to land onto SSA patches in a follow-up?
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
extended/src/main/java/io/kubernetes/client/extended/kubectl/KubectlRolloutHistory.java
Outdated
Show resolved
Hide resolved
eaf81e6
to
11f1878
Compare
11f1878
to
2957526
Compare
2957526
to
ade222f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
thanks!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: NeverRaR, yue9944882 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
feat : support kubectl rollout history for daemonset